This attempts to resolve issue #9646 (closed).
We can use DwmSetWindowAttribute () to set dialogue titlebars to either dark or light mode on Windows.
- For the main window, this is done by checking the
prefer-dark-themeGUI config property on start-up and when the theme is refreshed. - For other dialogues, this is done by checking the dialogue's background color and setting to dark mode if it's below a threshold (currently 0.5, 0.5, 0.5) since we don't have access to the GUI config property. I've added the function call to
GimpDialog,GimpFileDialog, andGimpAboutDialog(since it seems to be its own thing) which so far handles most dialogues.
The main caveat is that this won't update already open dialogue title bars until they're reopened. Since the only way this would happen is if you open Preferences while doing other work and make the change, I don't think it's worth the additional complexity (at least until we can do this for all OS and not just Windows).
@brunolopesdsilv: Hi! When you have time, could you test this and see what you think? I'm interested if there are any performance issues, graphical glitches, and if I've missed any dialogues (last one is an easy fix). Thanks!
@Jehan Hello! If you have time, can you give this a once-over and let me know if there are any structure issues or changes needed? I assume eventually we would want to use this same code for the MacOS title bars and maybe Linux (unless there's a better way already built into GTK), so let me know if I have wrong assumptions or need to re-arrange things. Thanks!
Merge request reports
Activity
92 va_list args); 93 94 gint gimp_dialog_run (GimpDialog *dialog); 95 95 96 96 void gimp_dialog_set_alternative_button_order_from_array 97 (GimpDialog *dialog, 98 gint n_buttons, 99 gint *order); 97 (GimpDialog *dialog, 98 gint n_buttons, 99 gint *order); 100 100 101 GBytes * gimp_dialog_get_native_handle (GimpDialog *dialog); 101 GBytes * gimp_dialog_get_native_handle (GimpDialog *dialog); 102 103 void gimp_dialog_set_title_bar_theme (GtkWidget *dialog); Hmmm… I don't get why make it a public function. It does seem to me like it is the obvious thing to do by default. Just make it a static function. If someone actually asks for it to be a public function and provides reasonable use case to support this request, we'll think if this is worth it.
@Jehan Ah, okay. I was thinking that theme makers might want to override the title bar, but since the current code doesn't allow for that anyway it's a moot point.
😄 However,
GimpAboutDialogno longer builds if I make the function static. I guess I'd need to change it to inherit from GimpDialog rather than GtkDialog?Yes, that's because I see you use
gimp_dialog_set_title_bar_themeinapp/dialogs/about-dialog.candapp/widgets/gimpfiledialog.c.One solution could be to create a
libgimpwidgets/gimpdialog-private.h, which would not be installed and put the function in there. It would still be exported (as long as you don't underscore the function name, which our build makes into an internal function), so it could be used by app/ code (as long as it's included there). Or simpler (though essentially the same thing): move it tolibgimpwidgets/gimpwidgets-private.h.This being said, you basically already have the same code inside
app(themes_set_title_bar()), and in fact an even better version as you don't have to guess if a theme is dark or not. You could just move this code intoapp/widgets/gimpwidgets-utils.[ch]with bothGimp *andGtkWidget *arguments. Insideapp/gui/themes.c, only keep the code looping through windows and make it call the utils function. In the about and file dialogs, also call this utils function.This second solution seems better than the first because you already have the code in app/ and with more theme knowledge.
I guess I'd need to change it to inherit from GimpDialog rather than GtkDialog?
Sure, it could be a third version, though having an utils function which works on every type of windows would be good too anyway.
@Jehan Thanks! If I understood you correctly, my function in
gimpwidgets-utils.cshould handle both possibilities (e.g. if we have access to the GIMP object we can get the config property, and if we don't we fallback to checking the background color). I updated the code based on that idea.
Looks fine to me, except that we should likely not have any public function. Just makes this happen on all GimpDialog and done.
This way, it is self-contained enough that it's not a problem if we need to update the logic (e.g. if we have a better solution than the "guessing dark mode" by background color) in the future or reorganize things.
Hi @cmyk.student . I'm still struggling to reconfigure my build setup, but I hope I'll be able to finally test it today.
(CI Windows builds without the installer are just useless since the .zip artifact somewhat don't have icons and scheme files that lead GIMP to not run)
Edited by Bruno Lopes420 427 } 421 428 } 422 429 430 static void 431 gimp_file_dialog_map (GimpFileDialog *dialog, 432 gpointer data) 433 { 434 #ifdef G_OS_WIN32 435 gimp_window_set_title_bar_theme (NULL, GTK_WIDGET (dialog)); @Jehan Thanks, fixed now!
206 208 207 209 dialog->timer = g_timeout_add (800, about_dialog_timer, dialog); 208 210 } 211 212 #ifdef G_OS_WIN32 213 gimp_window_set_title_bar_theme (NULL, widget); What about adding
Gimp *gimpas arg toabout_dialog_create()? I see it's used in 2 places:- In
app/dialogs/dialogs-constructors.cwhere we havecontext->gimp. - In
app/gimp-update.cwhere it would be extra-easy to makegimpobject the user_data ofgimp_update_about_dialog()(right now we set it as NULL).
Basically within
app/, we have access to this object. Let's just pass it through to get the correct info for sure, rather than guessing.- In
@Jehan Ah, I see. Okay, fixed as well.
2627 void 2628 gimp_window_set_title_bar_theme (Gimp *gimp, 2629 GtkWidget *dialog) 2630 { 2631 #ifdef G_OS_WIN32 2632 HWND hwnd; 2633 GdkWindow *window = NULL; 2634 GimpGuiConfig *config; 2635 gboolean use_dark_mode = FALSE; 2636 2637 window = gtk_widget_get_window (GTK_WIDGET (dialog)); 2638 if (window) 2639 { 2640 if (gimp) 2641 { 2642 config = GIMP_GUI_CONFIG (gimp->config); - Resolved by Alx Sa
If I understood you correctly, my function in
gimpwidgets-utils.cshould handle both possibilities (e.g. if we have access to the GIMP object we can get the config property, and if we don't we fallback to checking the background color).In fact, inside
app, we always have access to thegimpobject, at least so far in the pieces of code you touched (cf. my review). Maybe in some very specific cases, it would be more cumbersome to get the object (though probably never impossible). But in your current version of the patch, it's actually quite easy!I gave you instructions in my review for the 2 cases where you were passing NULL.
I was unable to compile either with the instructions on the developer's website or with the CI script. I feel very stupid, because I was able to do it both ways a few weeks ago.
Two types of errors happens:
- The deps libs aren't copied to the bin folder
- Even if i take needed .dll's from a 2.99 install, other error, like the CI builds are displayed: lack of icons and some scheme files
@brunolopesdsilv No worries! I need to make a further revision anyway based on Jehan's feedback, so maybe you'll be able to compile again before I finish.
added 1 commit
- aaf529ce - themes: Further revisions based on Jehan's feedback
52 54 { 53 55 GtkWidget *dialog; 54 56 57 Gimp *gimp; @Jehan Heh, yeah, I thought the original alignment was odd too. I'll fix it as well. :)
added 1 commit
- e0255810 - gui: Change Windows title bar based on theme
added 1 commit
- e2997e52 - gui: Change Windows title bar based on theme
added 3 commits
-
e2997e52...bf8ee695 - 2 commits from branch
master - ad8b47bf - gui: Change Windows title bar based on theme
-
e2997e52...bf8ee695 - 2 commits from branch